-
Notifications
You must be signed in to change notification settings - Fork 58
Change homepage masthead for Orange/Sosh #3241
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
✅ Deploy Preview for boosted ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
… colored bg examples)
MaxLardenois
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are more discussions than comments per se
site/src/scss/_masthead.scss
Outdated
| .bd-masthead-bg { | ||
| @include media-breakpoint-up(md) { | ||
| padding-right: 6rem; | ||
| @if ($brand == "orange") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't we have one version as the default? Otherwise we will have a if for each brands. Orange looks more generic with the bg image...
So if (sosh) {...} else {... // only orange for now}
Also we should convince the designers not to do something different for each brands imo...
| code={`npm i @ouds/web-common@${getConfig().current_version} @ouds/web-${getConfig().brand}@${getConfig().current_version}`} | ||
| lang="sh" | ||
| /> | ||
| <div class="bd-masthead-code mb-scaled-xsmall d-inline-block"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On mobile the copy button disappears and the text is not all visible nor accessible through scroll so it's not very useful. I would say hiding the npm install in the masthead on mobile can be ok because it will never be used.
The others in the page are at least selectable/scrollable so it's ok.
…e and do not display npm install command in the header in mobile mode
Types of change
Related issues
Partially fixes #3168
Fixes #3229
Context & Motivation
After adding the possibility to switch the brand from Orange to Sosh, change the masthead (below the header) of the homepage to add a new identity for Sosh
Description
-do not display the command
npm i @ouds/web...in the masthead in mobile mode because it cannot be fully displayed without any scrolling see [OUDS] Horizontal scrolling appears on the homepage under 488px width #3229Checklists
Checklist (for Core Team only)
Progression (for Core Team only)
ouds/mainfollowing conventional commitLive previews